feat(generate): port local-stack stack generation to Deno#19
Conversation
wax911
left a comment
wax911
left a comment
There was a problem hiding this comment.
Review notes against #4:
-
Path rewriting is incorrect for multi-service stacks.
generateSingleStack()rewrites every service usingsources[0]?.composeDir. In local-stack, each service lives in its own directory, so env-file and bind-mount paths for every service after the first can be rewritten relative to the wrong directory. -
The network name is hardcoded as
traefik-public. The issue requires the generated network block to come from config.traefik-publicis the local-stack default, not a universal stackctl assumption. -
Top-level volume metadata is lost. The local-stack Python generator preserves volume metadata such as explicit
name:overrides when emitting external volumes. This implementation emits only{ external: true }for collected names. -
The command currently discovers all stacks when
options.stacksis omitted, but it does not appear to honor configured stack names/order from.stackctl. Determinism should come from config first, discovery second. -
The generated YAML contains a generated header comment. That is fine for output readability, but ensure snapshot tests account for it and that sync comparisons do not drift against existing local-stack generated output unless the migration intentionally updates those files.
This is close in shape, but the path-rewrite bug is a correctness blocker for the actual local-stack repository.
wax911
changed the base branch from
main
to
feat/3-implement-config-init-and-profile-discovery
- Fix path rewriting for multi-service stacks (per-service compose dir) - Use config stack.network instead of hardcoded "traefik-public" - Preserve top-level volume metadata from original compose files - Honor config stack.names ordering before discovery - Add deterministic output with sorted keys and stable header - Add snapshot test for generation output
wax911
left a comment
wax911
left a comment
There was a problem hiding this comment.
Follow-up review after the push:
Good fixes:
- The multi-service path rewrite issue is addressed with a service-to-compose-directory map.
- The network name is now configurable through
GenerateOptions.networkinstead of being only hardcoded. - Existing top-level volume definitions are preserved and get
external: truewhen missing. - Config stack names are now accepted through
configStackNames.
Remaining issues:
-
The PR is currently not mergeable and its base SHA is behind the current config PR. Rebase it after #18 is fixed.
-
The default network still falls back to
traefik-public. That is fine as a local-stack default, but it should come from DEFAULT_CONFIG/config wiring rather than being a universal compose module default. If this is intentionally the built-in default, document it as such. -
Confirm snapshot compatibility against current local-stack generated stacks. The added generated header comment and key sorting may intentionally change output, but
syncmust not produce noisy drift during migration unless local-stack updates the committed stack artifacts in the same migration. -
Make sure the service-dir map handles duplicate service names across different compose sources deterministically. The first entry wins today; that should either be validated as an error or documented.
This is much closer. The previous correctness blocker is fixed, but mergeability and parity validation still need attention.
wax911
force-pushed
the
feat/3-implement-config-init-and-profile-discovery
branch
from
0438af4 to
8566b25
Compare
- Fix path rewriting for multi-service stacks (per-service compose dir) - Use config stack.network instead of hardcoded "traefik-public" - Preserve top-level volume metadata from original compose files - Honor config stack.names ordering before discovery - Add deterministic output with sorted keys and stable header - Add snapshot test for generation output
wax911
force-pushed
the
feat/4-port-stack-generation-to-deno
branch
from
1d6cee2 to
0c01b0f
Compare
1 participant
Closes #4